Skip to content

fix(keybindings): print Lua-compatible combos - #7660

Open
ketpatil77 wants to merge 5 commits into
omacom:quattrofrom
ketpatil77:fix/keybinding-print-format
Open

ketpatil77 wants to merge 5 commits into
omacom:quattrofrom
ketpatil77:fix/keybinding-print-format

Conversation

@ketpatil77

Copy link
Copy Markdown

What changed

Format every modifier separator in keybindings --print with +.

Printed bindings such as SUPER + SHIFT + S can now be copied directly into hl.unbind(...) or o.bind(...).

Closes #7627

Testing

  • exercised parse_binding_records with SUPER SHIFT,S and SHIFT ALT,D
  • git diff --check

Signed-off-by: ketpatil77 <243740572+ketpatil77@users.noreply.github.com>
@ketpatil77
ketpatil77 marked this pull request as ready for review August 25, 2026 11:00
Copilot AI balanced review requested due to automatic review settings August 25, 2026 11:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
omarchybot and others added 3 commits September 28, 2026 13:15
prioritize_entries recognised the secondary browser chord as "SUPER SHIFT ... B", the format --print used before this branch. With every modifier now joined by " + ", the pattern stopped matching, so SUPER + SHIFT + B took the primary Browser slot and sorted ahead of SUPER + SHIFT + RETURN.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The records cache is keyed on a version string, the active keymap and hyprctl binds, none of which change when the chord formatting does. Without a bump, an upgraded install keeps serving "SUPER SHIFT + B" from its cache to both --print and the menu until its bindings happen to change.

Co-Authored-By: Codex Medium <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@omarchybot omarchybot added the verified Omarchy Triage has verified that this issue is ready for final review label Sep 28, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed and verified on a disposable Omarchy worker (Hyprland 0.56.2). The problem in #7627 is real and this fixes it: hl.unbind("SUPER SHIFT + B"), the form --print gives on quattro, leaves the binding in place, and hl.unbind("SUPER + SHIFT + B"), the form it gives with this change, removes it.

I pushed three commits to the branch:

  • ee7b927 bumps the keybindings cache version. The cache is keyed on the version, the keymap and hyprctl binds, none of which change with the formatting, so an upgraded install kept serving SUPER SHIFT + B from cache to both --print and the menu. I confirmed that on the worker with a cache warmed by quattro.
  • 0393414 updates the two prioritize_entries patterns that looked for SUPER SHIFT ... B → Browser. With the new separators they stopped matching, and Super + Shift + B moved into the primary Browser slot ahead of Super + Shift + Return.
  • 8cd032b merges quattro, which had moved on since this was opened, so the cache version could be bumped from its current value without a conflict. The diff against quattro is still one file, +7/−5.

Tests on the worker: test/shell.d/keybindings-menu-test.sh passes 16 of 16. ./test/cli passes 88 of 89. The one failure, vscode generated theme references current theme file, fails the same way on quattro and is unrelated to this change.

The second opinion was Codex Medium, over two rounds. It found the stale cache, which the first commit above fixes, and its second round on the final head found nothing open. It also raised two points I've left for the maintainer rather than changed here. First, the menu's filter is a literal substring match, so typing super shift without the + no longer finds these rows. Second, a custom chord with four modifiers (e.g. SUPER + SHIFT + CTRL + ALT + BACKSPACE) now runs past the menu's 35-column alignment. No shipped default does. Separately, #11733 rewrites this same code and deliberately prints modifiers compacted (SUPER SHIFT + B), so the two conflict on the format.

Nothing is needed from you. This waits on the maintainer.

@omarchybot omarchybot removed the verified Omarchy Triage has verified that this issue is ready for final review label Sep 28, 2026
@greptile-apps

greptile-apps Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This PR appears safe to merge.

Summary

Formats multi-modifier shortcuts with + so individual combos match the form used in Lua bindings.

  • Printed key combos put + between every modifier.

Reviews (1) · Last reviewed commit: "Merge current quattro and refresh format..." · Reviewed by Greptile

@omarchybot omarchybot added verified Omarchy Triage has verified that this issue is ready for final review ready Good to merge labels Oct 15, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Re-checked at b6822a0, the merge of current quattro with the cache version moved to v18. Against quattro the change is still one file, bin/omarchy-menu-keybindings, +7/−5, and the merge brought nothing else into it.

Verified again on a disposable Omarchy worker (Hyprland 0.56.2). --print on quattro gives SUPER SHIFT + B, and hl.unbind of that leaves the binding in place. This head prints SUPER + SHIFT + B, and hl.unbind of that removes it. With the change reverted it fails again. A cache warmed by quattro is not reused: this head prints the new format straight away, and Super + Shift + Return stays in the primary Browser slot.

Tests on the worker: ./test/cli passes 125 of 125, test/shell.d/keybindings-menu-test.sh 20 of 20 and menu-keybindings-key-names-test.sh 3 of 3. i18n-test.sh fails one check, da/menu.json mirrors the English catalog, the same way on quattro; this change doesn't touch it.

The second opinion was Codex Medium. It reviewed this head and found nothing open. It agreed with my reading of the cache key, the Browser patterns and the menu's exact-match selection, though its independence isn't guaranteed. The points in my earlier comment are unchanged and still for the maintainer: the menu filter no longer matches super shift typed without the +, four-modifier chords run past the 35-column alignment, and #11733 prints modifiers in the compact form.

Nothing is needed from you. This waits on the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready Good to merge verified Omarchy Triage has verified that this issue is ready for final review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

keybindings --print uses a different format than hl.unbind / o.bind

3 participants